Review: assume coregister=True when like= is given

The change does what it says and the logic holds up across the kwarg combinations I worked through.

Blockers

None.

Suggestions

• xrspatial/geotiff/tests/test_xarray_backend_like_default_coregister_3379.py: the new tests all use a DataArray like. The default-coregister line runs before the DataArray-vs-Dataset dispatch, so a Dataset template hits the same code, but a one-line to_dataset() case would lock in that a Dataset like= alone also coregisters (the #3376 suite only covers Dataset like with explicit coregister=True).

Nits

None worth holding the PR for.

What looks good

• The guard if 'coregister' not in kwargs and not kwargs.get('auto_reproject') is the right shape. An explicit coregister= wins, an explicit auto_reproject=True keeps the lighter reproject, and auto_reproject=False still falls through to the coregister default. _xarray_backend.py:127.
• The no-like ValueError guard is untouched, so passing a coregister kwarg without a target still raises the pointed error.
• The behavior change (a bare like= used to be a plain windowed read) is intentional and is called out in both the module docstring and docs/source/reference/geotiff.rst, with coregister=False documented as the opt-out.
• Existing #3376 engine tests (auto_reproject parity, GPU/.vrt rejection, naming) still pass.

Checklist

• Logic correct across coregister / auto_reproject / neither
• Backend coverage inherited from the accessor (CPU-only coregister, GPU/.vrt rejected as before)
• No premature materialization (kwarg routing only)
• Behavior change documented
• Docstrings and reference docs updated
• Dataset like= alone not directly tested (see suggestion)

Follow-up review (after 9a58cdb)

The one suggestion from the first pass is addressed: test_dataset_like_alone_coregisters now covers a bare Dataset like= snapping onto its grid, alongside the DataArray cases. The follow-up is test-only, so no production code moved.

No new blockers, suggestions, or nits. Full file (7 cases) passes locally.

Review: redesign — target rides on coregister=/auto_reproject= (drop like=)

Reviewed the reworked branch (41c3bce). The routing holds up across the kwarg combinations.

Blockers

None.

Suggestions

None.

Nits

• _xarray_backend.py: if a caller passes a grid on coregister= and a bool auto_reproject=True (not an array), coregister wins silently and auto_reproject=True is forwarded but ignored by the accessor's precedence. Harmless and an odd thing to write, so not worth a guard — noting it for the record.

What looks good

• The array form is detected with isinstance(x, (xr.DataArray, xr.Dataset)) and rewritten to a plain True before delegating, so the accessor's if coregister: logic never sees an ambiguous array. _xarray_backend.py open_dataset. The accessor file is untouched and its coregister suite (test_open_geotiff_coregister.py) still passes — confirmed locally.
• The no-target guard uses kwargs.get(k) truthiness, which is safe because that branch only runs when neither mode carries an array; bare coregister=True, lone resampling/var, the both-modes conflict, and the removed like= all raise pointed errors (covered by tests).
• var= and resampling= ride through as modifiers of a Dataset/array target; Dataset dispatch + var= is tested.
• Error messages name the replacement, so a day-one like= user gets a clear migration path rather than an opaque TypeError.

Checklist

• Truthiness contained at the engine boundary; accessor unchanged
• All error paths covered (removed like=, bare bool, lone modifier, both modes)
• coregister/auto_reproject parity with the accessor
• open_mfdataset shares the grid with coregister=
• docs (geotiff.rst + module docstring) rewritten to the new API
• accessor regression suite green

Review: tight pass on the redesign (1993eb2)

One real bug, one nit, otherwise clean.

Blockers

• _xarray_backend.py open_dataset, no-target branch: a falsy coregister / auto_reproject passed alone leaks into the standalone reader. The guard switched from k in kwargs to kwargs.get(k), so backend_kwargs={"coregister": False} (also False/None on auto_reproject) is not flagged, stays in kwargs, and reaches open_geotiff(filename_or_obj, **kwargs) — which has no coregister parameter. Result: TypeError: open_geotiff() got an unexpected keyword argument 'coregister', the opaque failure this engine is meant to turn into a pointed error. coregister=False should mean "plain read", so strip the falsy mode flags before the standalone call rather than forwarding them:

  for k in _COREGISTER_ONLY_KWARGS:
      if not kwargs.get(k):
          kwargs.pop(k, None)
  da = open_geotiff(filename_or_obj, **kwargs)

Nits

• Same method: a bool coregister=True combined with an array auto_reproject=<grid> (or vice versa) takes the array as the target, sets that mode, and forwards the leftover True on the other flag; the accessor's coregister-precedence then silently wins. It's a contradictory input no one should write, so a guard is probably not worth it — flagging for the record.

What looks good

• Truthiness is contained at the engine boundary: isinstance picks the array target and the accessor only ever sees a plain bool. The accessor file is untouched and its suite (test_open_geotiff_coregister.py) still passes.
• Error coverage is solid for the truthy cases: removed like=, bare True modes, lone resampling/var, and a target on both modes all raise pointed errors with tests.
• var=/resampling= ride through as modifiers; Dataset dispatch + var= is tested; open_mfdataset shares the grid.

Coverage gap (tied to the blocker)

• No test exercises a falsy mode flag alone. Add one asserting backend_kwargs={"coregister": False} returns a plain read equal to the no-backend-kwargs read — it would have caught this.

Checklist

• Truthiness contained; accessor unchanged
• Falsy mode flag alone handled (blocker above)
• Truthy error paths covered
• Parity with the accessor for both modes
• Docs rewritten to the new API

Follow-up: blocker fixed (d4ed9f7)

The falsy-mode-flag leak is closed. The no-target branch now pops the mode/modifier kwargs before the standalone read, so coregister=False / auto_reproject=False / coregister=None do a plain read instead of leaking an opaque TypeError. Verified False, auto_reproject=False, and None all return the same array as a no-backend-kwargs read.

Added test_falsy_mode_flag_alone_reads_plain (parametrized over both flags) asserting parity with the plain read. 53 engine + accessor tests pass, accessor suite unchanged.

The nit (contradictory bool-True + array on the other mode) is left as-is — a contrived input not worth a guard.